Skip to content

[SPARK-59325][SQL] Add an optional maximum nesting depth for variant values - #58607

Closed
HyukjinKwon wants to merge 1 commit into
apache:masterfrom
HyukjinKwon:SPARK-59325-variant-max-depth
Closed

[SPARK-59325][SQL] Add an optional maximum nesting depth for variant values#58607
HyukjinKwon wants to merge 1 commit into
apache:masterfrom
HyukjinKwon:SPARK-59325-variant-max-depth

Conversation

@HyukjinKwon

@HyukjinKwon HyukjinKwon commented Sep 8, 2026

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

Add an opt-in spark.sql.variant.maxNestingDepth (internal, default -1 = unlimited = unchanged).
When set to a positive value, rendering a variant value whose nesting exceeds it fails instead of
recursing without bound. The limit is threaded as a parameter into the recursive variant read
paths so common/variant does not need SQLConf access:

  • Variant.toJson / toJsonImpl and VariantVal.toJson gain overloads carrying the limit; the
    existing signatures delegate with -1 (unchanged).
  • The SQL cast/variant_get-to-string path, the Parquet variant shredding read, and
    to_json obtain the limit once (per expression / per task), not per row.

Why are the changes needed?

Variant.toJsonImpl recursed per nested element with no bound, so a deeply nested variant could
exhaust the stack when rendered. The existing size limit does not prevent this (a small-per-level
variant can nest very deeply within the size cap). This adds an optional bound.

Does this PR introduce any user-facing change?

No by default. When spark.sql.variant.maxNestingDepth is set to a positive value, variant values
nested more deeply than the limit raise an error when rendered.

How was this patch tested?

New VariantExpressionSuite case: a deeply nested variant renders fully when the limit is unset or
generous and is rejected when the limit is smaller than its depth.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Isaac

This pull request and its description were written by Isaac.

Co-authored-by: Isaac no-reply@databricks.com

@HyukjinKwon
HyukjinKwon force-pushed the SPARK-59325-variant-max-depth branch 2 times, most recently from a7061fa to 26b96a0 Compare September 8, 2026 22:32
@HyukjinKwon Isaac
[SPARK-59325][SQL] Add an optional maximum nesting depth for variant …
d784c05 
…values

### What changes were proposed in this pull request?

Add an opt-in `spark.sql.variant.maxNestingDepth` (internal, default `-1` = unlimited = unchanged).
When set to a positive value, rendering a variant value whose nesting exceeds it fails instead of
recursing without bound. The limit is threaded as a parameter into the recursive variant read
paths so `common/variant` does not need `SQLConf` access:
- `Variant.toJson` / `toJsonImpl` and `VariantVal.toJson` gain overloads carrying the limit; the
  existing signatures delegate with `-1` (unchanged).
- The SQL cast/`variant_get`-to-string path, the Parquet variant shredding read, and
  `to_json` obtain the limit once (per expression / per task), not per row.

### Why are the changes needed?

`Variant.toJsonImpl` recursed per nested element with no bound, so a deeply nested variant could
exhaust the stack when rendered. The existing size limit does not prevent this (a small-per-level
variant can nest very deeply within the size cap). This adds an optional bound.

### Does this PR introduce _any_ user-facing change?

No by default. When `spark.sql.variant.maxNestingDepth` is set to a positive value, variant values
nested more deeply than the limit raise an error when rendered.

### How was this patch tested?

New `VariantExpressionSuite` case: a deeply nested variant renders fully when the limit is unset or
generous and is rejected when the limit is smaller than its depth.

### Was this patch authored or co-authored using generative AI tooling?

Generated-by: Isaac

This pull request and its description were written by Isaac.

Co-authored-by: Isaac <no-reply@databricks.com>
@HyukjinKwon
HyukjinKwon force-pushed the SPARK-59325-variant-max-depth branch from 26b96a0 to d784c05 Compare September 9, 2026 11:00
@HyukjinKwon

HyukjinKwon commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

Closing this.

I looked into whether the depth limit is actually reachable for engine-produced variants, and it isn't:

  • Every JSON -> variant path (parse_json, from_json, ...) goes through VariantBuilder, which uses VariantUtil.JSON_FACTORY = new JsonFactory(). Jackson's default StreamReadConstraints caps nesting at 1000, so parsing rejects anything deeper at parse time:

    depth 1000:  OK
depth 1001:  StreamConstraintsException: Document nesting depth (1001) exceeds
             the maximum allowed (1000, from StreamReadConstraints.getMaxNestingDepth())

  • VariantBuilder.buildJson is itself recursive (per START_OBJECT/START_ARRAY), so even without the Jackson cap it would overflow at build time -- you can't materialize a variant deep enough to later overflow toJson.

  • toJson's recursion only overflows in the low-thousands+, i.e. above the 1000 producer cap. So a variant the engine builds can't reach the toJson overflow.

The only way to get a variant deeper than that into toJson is a raw variant binary read from storage that never went through the builder (a corrupt or hand-crafted blob). That's a narrow malformed-input hardening case, and toJson isn't the only recursive consumer of such bytes (cast, variant_get, shredding recurse too), so guarding toJson alone wouldn't be a complete answer there anyway.

Given that the guard is unreachable on the normal engine paths, the added config + toJson overload + VariantCastArgs plumbing isn't justified. If we later want read-side defense-in-depth against untrusted stored binaries, a small fixed internal cap mirroring the parser's 1000 would be the way, without any new API. Closing for now.

@HyukjinKwon HyukjinKwon closed this Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@HyukjinKwon